Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DRAFT] gh-105059: C99 avoids PyObject.ob_refcnt union #105767

Closed
wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jun 14, 2023

Only define PyObject.ob_refcnt as an anonymous union on C11 and newer, or when GCC is used. If GCC is used on C99 and older, add extension on the union. On C99 and older without GCC, Py_INCREF() and Py_DECREF() are implemented as function calls.

Only define PyObject.ob_refcnt as an anonymous union on C11 and
newer, or when GCC is used. If GCC is used on C99 and older, add
__extension__ on the union. On C99 and older without GCC, Py_INCREF()
and Py_DECREF() are implemented as function calls.
@vstinner
Copy link
Member Author

vstinner commented Jun 14, 2023

When the Python C API is used with GCC and clang, I'm pretty sure that __extension__ can be used to get an anonymous union, without emitting new compiler warnings or errors, even when -pedantic option is used. For now, I prefer to skip this code path to focus on C11 vs C99 code path, so I commented the code (you can uncomment it to test).

I don't know if MSC has pragma to turn off the compiler warning on "Compiler Warning (level 4) C4201": "nonstandard extension used: nameless struct/union".

@vstinner
Copy link
Member Author

For now, my PR implements Py_INCREF() and Py_DECREF() as opaque function calls on C99 and newer. Maybe later, this code path can still be optimized to inline the code. But a solution without anonymous union should be found for that.

@vstinner
Copy link
Member Author

To test this PR, I added the following code to Modules/_testcapimodule.c and Programs/_testembed.c:

#ifdef _PYOBJECT_REFCNT_ANON_UNION
    printf("PyObject.ob_refcnt is an union\n");
#else
    printf("PyObject.ob_refcnt type is Py_ssize_t\n");
#endif

And I modified Makefile.pre.in to build Programs/_testembed.c with -std=c99: in this case, the union is not used, whereas _testcapi is built with the union (it's built with -std=c11).

@vstinner
Copy link
Member Author

This change can have a significant negative impact on performance, so I abandon it and instead try to get rid of the compiler warning: PR #107232.

@vstinner vstinner closed this Jul 25, 2023
@vstinner vstinner deleted the incref_c11 branch September 13, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant